I was going to suggest adding a method like Storage::init_view() that didn't take any params and set both Option::nones under the hood. But if we're going to hide this behind a macro, no need to polish it up, right?

Yes, I think it doesn't make sense to implement because of the macro. It would just make Storage a bit more complex and the gain would not be there.

Did you run a code formatter or something? If so, how? We should get it into the CI so we ensure all our code gets formatted.

I used rustfmt but it was a semi-manual process because rustfmt fails on unconstrained methods and global vars. How I made it work is that I first renamed unconstrained get_balance to get_balance_unconstrained and removed the global vars and then reverted the changes. The formatting became too ugly so it was worth the hassle but this makes it impossible to use in CI without bigger modifications of rustfmt.

But this actually makes me curious whether to create noirfmt it would be sufficient to create some simple pre and post processors which would make and revert the changes I did manually... 🤔

FWIW I found a tracking issue in Noir, which doesn't seem prioritised atm: noir-lang/noir#1580

As for using rustfmt, it's an interesting idea, but I'm worried that it'll keep diverging as we add more non-rust features to Noir. Also, removing noir-specific keywords is easy, but adding them back is not, given the file changes introduced by rustfmt. I think we should probably wait.

Nit: avoid using variable names starting with underscore, since they are (or are in most languages, not sure if Rust also) skipped for unused variable checks. As an alternative, you can rename the field to maybeContext, and the unwrapped version to context.

I'm on the fence about this: should we pass both optional contexts to every state var type? Or just the ones they actually need, like in ImmutableSingleton? Still, when we introduce traits, this problem will go away.

I hadn't appreciated how ugly storage declarations would become with this new change. Declaring storage like this doesn't make for a very nice user experience, does it? (I know I initially proposed the change, to clean up calls to methods (by removing the context as a parameter to those calls)... but this is an ugly trade off, and I'm not sure if it's a good one, in hindsight).
I returned to this PR after a breaking change was flagged on discord @benesjan @spalladino - this PR caused a breaking change to storage declarations, and someone was struggling with declaring a double-mapping with this new syntax (and I can understand why :) )

Declaring a double mapping has become this monstrosity, haha:

Storage {
    double_mapping: Map::new(
        private_context,
        public_context,
        1,
        |private_context, public_context, slot1| Map::new(
            private_context,
            public_context,
            slot1,
            |private_context, public_context, slot2| Set::new(
                private_context,
                public_context,
                slot2,
                ValueNoteMethods
            )
        )
    )
}

This might have to be classified as "unacceptable syntax"?

Edit: especially when compared to: mapping(uint256 => mapping(uint256 => uint256)) doubleMapping; in Solidity.

Agree it's horrible, but bear in mind that, as soon as we get traits, we can work with a single context. So the example above would become this. And we can even introduce a DoubleMap struct that abstracts it if we wanted to.

Storage {
    double_mapping: Map::new(context, 1,
        |context, slot1| Map::new(context, slot1, 
            |context, slot2| Set::new(context, slot2, ValueNoteMethods)
        )
    )
}

I still think the change is worth it. I'd expect developers to spend more time in contract logic that in storage declaration. And by moving the context into storage initialisation (which we can then hide with a macro), we can keep moving explicit references to context out of the picture, for cleaner contract code.

TLDR: Can you push the Noir team to get traits implemented soon? :-P

I don't think they'll be implemented for some weeks (at the earliest). Perhaps we could wrap the private_context, public_context "tuple" in a Context type, just to reduce the verbosity of this.

struct Context {
    private_context: Option<PrivateContext>,
    public_context: Option<PublicContext>,
    _is_private: bool
}

And then pass this ugly thing between state variables instead?

A double mapping is a nice idea too!

TLDR: Can you push the Noir team to get traits implemented soon? :-P

I just asked, for you:

Perhaps we could wrap the private_context, public_context "tuple" in a Context type, just to reduce the verbosity of this.

I think that sould be easy to do (annoying, but easy) until we do get traits. Do you want to create an issue and assign it to Otters?